-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix/cleanup api #10
Merged
Merged
fix/cleanup api #10
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mimir-d
force-pushed
the
fix/cleanup_api
branch
from
October 8, 2024 17:24
70167d9
to
1e73540
Compare
njordr
approved these changes
Oct 9, 2024
mimir-d
force-pushed
the
fix/test_incorrect_asserts
branch
from
October 9, 2024 12:59
2814f15
to
b5f9fe2
Compare
- this is public api; consolidate with public config types - the emitter is not public, so it sits in its own file Signed-off-by: mimir-d <[email protected]>
- add a single error type for most of the api Signed-off-by: mimir-d <[email protected]>
- remove WriterError because it'll always be a type of io::Error - add WriterType::Custom so that we can write a test checking an actual error return from an api call (missing before this commit) Signed-off-by: mimir-d <[email protected]>
Signed-off-by: mimir-d <[email protected]>
- this change disallows usage of contextual objects after emitting their end artifact, at compile time - previously a user could have emitted, for example, a step log after a step end Signed-off-by: mimir-d <[email protected]>
- some methods had a verb, some didnt; add a verb to all the public api methods for consistency (also matches the python api more) - another reason for this is that the verb allows for backward/future compat in the api (eg. `add_step` could turn into `start_step`, with the semantic of also starting the step, not just creating it; all while being able to keep the previous `add_step` name. if the method was called just `step`, this wouldnt be possible) Signed-off-by: mimir-d <[email protected]>
mimir-d
force-pushed
the
fix/cleanup_api
branch
from
October 9, 2024 13:04
1e73540
to
71bcce6
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
error return from an api call (missing before this commit)
end artifact, at compile time
methods for consistency